Independent code review

Posted on behalf of @ds4psb-ai (read-only contributor). Maintainer judgment owns merge.

Summary verdict

LGTM — small, surgical, evidence-backed fix; no blockers.

Findings

P0 / blocker

• (none)

P1 / should fix before merge

• (none)

P2 / nice to have / followup

• src/clawsweeper.ts:3609 isCommitSha — the regex /^[0-9a-f]{7,40}$/i is correct, but the existing reportLikelyOwners parser at src/clawsweeper.ts:4313-4319 already trims commits when reading them out of stored markdown. The new .map(commit => commit.trim()) is therefore redundant for the persisted-then-rendered path, but is necessary for the freshly-decided path (object straight from Codex JSON). Worth a one-line code comment so future readers don't strip the trim on the assumption it's dead code.
• src/clawsweeper.ts:4097-4101 — consider also dropping owners whose entire commits array filters to empty after the SHA gate (currently they still render with no commits: suffix, which is the correct/safe behavior — flagging only because the PR description focuses on URL noise and you may want symmetry with how empty files is handled). Not blocking.

Test coverage

The single new test at test/clawsweeper.test.ts:629-652 does cover the mixed-provenance case the PR body claims (one URL + one valid SHA, with whitespace padding on the SHA), and asserts both negatives (no /commit/https: substring) and positives (the well-formed link). Good shape. Two coverage gaps worth considering (P2):

1. A pure-noise input: commits: ["#23580", "PR #67960"] — should produce a line with no commits: suffix at all. This is the exact pattern from the issue 72080 and PR 71158 evidence URLs in the description.
2. Uppercase-hex SHA acceptance — the regex uses the i flag and your isCommitSha unit-tests aren't exposed; one round-trip case would lock that in.

Neither is blocking; the existing test catches the production regression class.

Risks I considered and dismissed

• False negatives on real short SHAs: regex floor of 7 matches GitHub's own short-SHA convention, no risk.
• Truncated 6-char SHAs from older Codex output: searched src/clawsweeper.ts for shortSha( callers — nothing emits a 6-char value back into a likelyOwner commits field, so the 7-char floor is safe.
• Unicode / smart-quote variants of PR #...: not produced by Codex; not in the evidence URLs.
• Production evidence still broken: spot-checked openclaw/openclaw#72080 (still renders /commit/#23580 as a link) and openclaw/openclaw#71158 (still renders /commit/PR links) — confirms the fix isn't already obsoleted by something else.

Nice catch on the trim — many "filter Boolean" guards miss whitespace-only strings.